Improve accessibility of search form #188
Conversation
Why these changes are being introduced: Accessibility testing revealed that the search button is difficult for screen reader users to locate, and that the `title` attribute for the keyword search input is not useful. Relevant ticket(s): * [GDT-318](https://mitlibraries.atlassian.net/browse/GDT-318) How this addresses that need: This changes the keyword `title` to 'keyword anywhere' and moves the search button to the bottom of the form, after all of the advanced search dropdowns. It also updates some corresponding CSS, including removing the `border-radius` override for the `btn` class, so the search button looks more like a button. Side effects of this change: Fixed margin/padding for results summary panel and mobile filter toggle. (This is unrelated to this change, but I noticed while testing out the changes that the spacing looked odd.)
Pull Request Test Coverage Report for Build 9019155779Details
💛 - Coveralls |
| class="field field-text basic-search-input <%= "required" if search_required %>" name="q" title="<%= label %>" | ||
| placeholder="Enter your search" | ||
| class="field field-text basic-search-input <%= "required" if search_required %>" name="q" | ||
| title="Keyword anywhere" placeholder="Enter your search" |
There was a problem hiding this comment.
I hadn't noticed previously that when we expand to show additional form fields we change the placeholder text for this field. I understand why we do that, but it feels a bit odd and we may want to revisit (later) how to handle this (including possibly just throwing "advanced search" into it's own view which I don't love but does make things simpler for a few edge cases).
There was a problem hiding this comment.
We used to do it for all of the dropdowns as well. I think it's bad practice (usability testing confirmed that suspicion), and I'm taking this opportunity to remove the last instance of it.
There was a problem hiding this comment.
Can you clarify what you commented here? I still see this behavior on this PR for all of the various expandable form components. Are you referring to something other than the placeholder for keyword searching changing when expanding/collapsing?
There was a problem hiding this comment.
Oh, I was actually talking about labels, not placeholder text. We used to change the text of the dropdown labels when they were open (e.g., "Geospatial bounding box search" became "Close geospatial bounding box search"). The keyword input label was the last instance of that, changed here.
I don't care as much about placeholder text, but I'd be perfectly happy to leave it as 'Keyword anywhere' in all form states. I'm not sure 'Enter your search' is adding any value here.
Sorry for the confusion; I only realized after your question how confusing my comment was. 🙂
There was a problem hiding this comment.
Gotcha, I see what you were referring to and yes that has been a good change.
I don't feel strongly enough about the placeholder text to override any previous decisions on it. I find it unexpected, but it isn't inherently wrong.
There was a problem hiding this comment.
Not inherently wrong, no, but I get the sense that 'Keyword anywhere' is more useful regardless of form state. (But, like you, probably not enough to make a change here.)
|
FYI, I requested review from Darcy on the PR app, so I'm waiting to merge until after I hear from her. |
|
UXWS has approved this change. |
Why these changes are being introduced:
Accessibility testing revealed that the search button is difficult for screen reader users to locate, and that the
titleattribute for the keyword search input is not useful.Relevant ticket(s):
How this addresses that need:
This changes the keyword
titleto 'keyword anywhere' and moves the search button to the bottom of the form, after all of the advanced search dropdowns. It also updates some corresponding CSS, including removing theborder-radiusoverride for thebtnclass, so the search button looks more like a button.Side effects of this change:
Fixed margin/padding for results summary panel and mobile filter toggle. (This is unrelated to this change, but I noticed while testing out the changes that the spacing looked odd.)
Developer
Accessibility
New ENV
Approval beyond code review
Additional context needed to review
UXWS will do this as part of QA, but it may be worth checking that everything looks okay in a smaller viewport.
Code Reviewer
Code
added technical debt.
Documentation
(not just this pull request message).
Testing